fix(auth): lower regional access boundary logs from warning to debug.#17571
Conversation
When the library tries to look up Regional Access Boundaries in the background and fails, it used to print a "WARNING" message. Because these lookups are optional and failing is not considered fatal or blocking, omitting warnings creates a lot of log noise for the users. This change lowers those messages to the "INFO" level so they stay out of the way by default.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request changes various Regional Access Boundary (RAB) lookup log messages from warning to info level and updates the test suite to verify this behavior. The reviewer pointed out that promoting the successful lookup log from DEBUG to INFO increases log noise for the happy path, which contradicts the goal of reducing noise, and suggested keeping it at DEBUG.
nbayati
changed the title
| e, | ||
| exc_info=True, | ||
| ) | ||
| adapter_type = " asynchronous " if is_async else " " |
There was a problem hiding this comment.
Why did we get rid of the is_logging_enabled check here? Was that intentional?
There was a problem hiding this comment.
Yes it was intentional. Previously, these were logged a WARNING. Because warnings are printed by default, we had to manually wrap it in that if check (which checks for DEBUG level) to hide it from normal users.
Now that we are logging them as _LOGGER.debug, they are already hidden by default. The logging library automatically filters out DEBUG logs, so we no longer need to write a manual if check to hide them.
| is_cloned, | ||
| ) = _prepare_async_lookup_callable(request) | ||
| except Exception as e: | ||
| if _helpers.is_logging_enabled(_LOGGER): |
There was a problem hiding this comment.
Ditto on the removal of this check
| ) | ||
| ) | ||
| except Exception as e: | ||
| if _helpers.is_logging_enabled(_LOGGER): |
| "Regional Access Boundary lookup failed. Entering cooldown." | ||
| in rab_caplog.text | ||
| ) | ||
| # RAB failures should be logged at INFO level. |
There was a problem hiding this comment.
INFO level comment is now obsolete it seems
There was a problem hiding this comment.
Thanks for the catch. I updated both the comment and the unit test to be more precise.
macastelaz
left a comment
macastelaz
left a comment
There was a problem hiding this comment.
Looks like there may be a need to re-format (lint error). Otherwise LGTM.
2 participants
When the library tries to look up Regional Access Boundaries in the background and fails, it used to print a "WARNING" message. Because these lookups are optional and failing is not considered fatal or blocking, omitting warnings creates a lot of log noise for the users. This change lowers those messages to the "DEBUG" level so they stay out of the way by default.
fixes #17515